file-system: drop redundant quotes around PathFmt and assert relative paths#15329
Conversation
github-actions
bot
added
the
store
amaanq
force-pushed
the
path-review-followup
branch
from
9387dd6 to
928de25
Compare
github-actions
bot
added
new-cli
Ericson2314
force-pushed
the
path-review-followup
branch
2 times, most recently
from
185f679 to
369d09b
Compare
| { | ||
| assert(isInStore(storePath)); | ||
| return getRealStoreDir() + "/" + std::string(storePath, storeDir.size() + 1); | ||
| return (getRealStoreDir() / std::string(storePath, storeDir.size() + 1)).string(); |
There was a problem hiding this comment.
Seems quite janky. What if storePath has some trailing slashes after storeDir? This would discard the LHS.
There was a problem hiding this comment.
I think storePath is always canonical here, because it comes from printStorePath which produces storeDir + "/" + storePath.to_string(), so there shouldn't be double or trailing slashes. The StorePath itself was validated in parseStorePath, which calls canonPath
There was a problem hiding this comment.
Any reason we can't accept StorePath here? I guess it would be a rather big change though...
There was a problem hiding this comment.
Yeah we could though I guess it would be a much larger change
There was a problem hiding this comment.
Or maybe not, because there's one call by derivation-builder.cc on line 1866 that passes in finalDestPath + ".check"..
There was a problem hiding this comment.
Ah right, it's not exactly a valid store path I suppose, but maybe we can hack it in in a way that's not that janky
There was a problem hiding this comment.
Done, made it a separate commit
There was a problem hiding this comment.
Oh this is very nice outcome!
- file-system: drop redundant quotes around `PathFmt` and assert relative paths - libutil, libstore: fix mingw cross-compilation breakages
amaanq
force-pushed
the
path-review-followup
branch
from
369d09b to
f4dfbca
Compare
amaanq
force-pushed
the
path-review-followup
branch
from
c27a99b to
d5dafb3
Compare
| mkstemp(const_cast<char *>(tmpl.c_str())) | ||
| #endif | ||
| ); | ||
| // `mkstemp` modifies the string to contain the actual filename. |
3 participants
Motivation
Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.